Require explicit model_name for SWE-Bench converters #376
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
|
@juanmichelini I think model_name: str = "OpenHands" is a bit misleading since the real values are llm names so I figured we could make it mandatory |
Co-authored-by: openhands <openhands@all-hands.dev>
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
Co-authored-by: openhands <openhands@all-hands.dev>
juanmichelini
left a comment
juanmichelini
left a comment
There was a problem hiding this comment.
Model name should include OpenHands prefix, this is necessary to share eval files with third parties.
| "instance_id": "django__django-11333", | ||
| "model_patch": "diff --git a/file.py b/file.py\n...", | ||
| "model_name_or_path": "OpenHands" | ||
| "model_name_or_path": "litellm_proxy/claude-sonnet-4-5-20250929" |
There was a problem hiding this comment.
Model name should include OpenHands, it can also include LLM model.
There was a problem hiding this comment.
OK. But I asked an agent "what are the actualy values that are given to this function given our CI framework" and those are the values. So we need to fix something I guess
There was a problem hiding this comment.
I guess this function is not used the way it's intended to be used ?
There should be no argument model_name or model_name_or_path, and "OpenHands" should just what's written there?
There was a problem hiding this comment.
So i guess my question is: is it even useful to add the llm name to the string here? Or can that method just insert "OpenHands" ?
In which case we can remove model_name for the method argument and just use the hardcoded "OpenHands"?
Summary
Testing